Skip to content

Introduce Spring integration tests #2205

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 1, 2023

Conversation

IlyaMuravjov
Copy link
Collaborator

Description

Introduce Spring integration tests that start SpringApplication and autowire tested class instance.

How to test

Manual tests

Generate tests for spring-boot-testing-main after putting the following in the application.yml.

spring:
  datasource:
    url: jdbc:h2:mem:testdb
    username: sa
    password: password
  jpa:
    hibernate:
      ddl-auto: create-drop
    show-sql: true
    generate-ddl: true

Self-check list

  • I've set the proper labels for my PR (at least, for category and component).
  • PR title and description are clear and intelligible.
  • I've added enough comments to my code, particularly in hard-to-understand areas.
  • The functionality I've repaired, changed or added is covered with automated tests.
  • Manual tests have been provided optionally.
  • The documentation for the functionality I've been working on is up-to-date.

@IlyaMuravjov IlyaMuravjov added ctg-enhancement New feature, improvement or change request comp-fuzzing Issue is related to the fuzzing comp-spring Issue is related to Spring projects support labels May 15, 2023
@EgorkaKulikov EgorkaKulikov force-pushed the ilya_m/spring_integration_test_prototype branch 2 times, most recently from cd12c2b to 1448cc9 Compare May 16, 2023 07:29
@IlyaMuravjov IlyaMuravjov force-pushed the ilya_m/spring_integration_test_prototype branch 2 times, most recently from 6dd562d to e295705 Compare May 21, 2023 14:26
Copy link
Member

@sergeypospelov sergeypospelov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utbot-instrumentation lgtm

val classId: ClassId,
val generics: List<FuzzedType> = emptyList(),
) {
open val usesCustomValueProvider get() = false
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inversion of dependencies between classes. FuzzedType should know nothing about how it is used.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something like hasCustomCreationMechanism could be a better name. The alternative is to make ObjectValueProvider aware of AutowiredFuzzed, but it seems more wrong to me.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The better way I see is to use marker interface to implement, like:

interface CustomValueProvider {
    fun getProvider(): ValueProvider
}

FuzzerType itself shouldn't implement that interface, but AutowiredFuzzerType should.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea, but I think this interface should be called CustomJavaValueProviderHolder with val property instead of the getter. In case of AutowiredFuzzedType, javaValueProvider can be injected into it via constructor by SpringFuzzedTypeFactory so we avoid cyclic dependency between AutowiredFuzzedType and AutowiredValueProvider.

Finally, we can add DelegatingToCustomJavaValueProvider to defaultValueProviders.

interface CustomJavaValueProviderHolder {
    val javaValueProvider: JavaValueProvider
}

object DelegatingToCustomJavaValueProvider {
    fun accept(type: FuzzedType): Boolean = type is CustomJavaValueProviderHolder

    fun generate(description: FuzzedDescription, type: FuzzedType): Sequence<Seed<FuzzedType, FuzzedValue>> =
        (type as CustomJavaValueProviderHolder).javaValueProvider.generate(description, type)
}

There's nothing Java specific about the code above, but if we use generic ValueProviderHolder<T, R, D : Description<T>> and DelegatingToCustomValueProvider<T, R, D : Description<T>> then unsafe cast to ValueProviderHolder<T, R, D> is needed instead of cast to CustomJavaValueProviderHolder.

@@ -54,6 +54,7 @@ suspend fun runJavaFuzzing(
constants: Collection<FuzzedConcreteValue>,
names: List<String>,
providers: List<ValueProvider<FuzzedType, FuzzedValue, FuzzedDescription>> = defaultValueProviders(idGenerator),
thisInstanceFuzzedTypeWrapper: (FuzzedType) -> FuzzedType = { it },
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look likes a workaround for a particular problem. Please, consider making it in more a general way. Also, the cache is used to keep types correctly, but know it is possible to make infinite recursive definition. Please, check that example like this method works correctly in test for the type resolving.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can accept an instance of FuzzedTypeFactory:

interface FuzzedTypeFactory {
    fun createFuzzedType(type: Type, isThisInstance: Boolean)
}

It will have two implementations:

  • PureJvmFuzzedTypeFactory traverses types recursively and incapsulates everything cache-related
  • SpringFuzzedTypeFactory is a decorator that makes this instance type to be autowired

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is good idea to use factory with decorators in this case 👍

is ReplaceIfPossible -> false
}
SpringTestType.INTEGRATION_TESTS -> true
}
else -> UtSettings.useFuzzing
}
val rdGenerateResult = process.generate(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it is time to write here smth like this:

val rdGenerateResult = process.generate(
    conflictTriggers = model.conflictTriggers,
    methods = methods,
    mockStrategyApi = model.mockStrategy,
    chosenClassesToMockAlways = model.chosenClassesToMockAlways,
    timeout = model.timeout,
    generationTimeout = model.timeout,
    isSymbolicEngineEnabled = false,
    isFuzzingEnabled = true,
    fuzzingValue = project.service<Settings>().fuzzingValue,
    searchDirectory = searchDirectory.pathString
)

@Markoutte Markoutte self-requested a review May 29, 2023 06:53
Copy link
Collaborator

@Markoutte Markoutte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed with @EgorkaKulikov we can rebase it into the main, because it doesn't break general fuzzing, but we still need to refactor the Spring part in fuzzing module with @IlyaMuravjov to improve some concepts.

@EgorkaKulikov EgorkaKulikov force-pushed the ilya_m/spring_integration_test_prototype branch 2 times, most recently from edaf990 to 81e364d Compare June 1, 2023 10:06
@EgorkaKulikov EgorkaKulikov marked this pull request as ready for review June 1, 2023 10:22
@EgorkaKulikov EgorkaKulikov enabled auto-merge (squash) June 1, 2023 10:48
@EgorkaKulikov EgorkaKulikov force-pushed the ilya_m/spring_integration_test_prototype branch from 3ea574f to 02607a1 Compare June 1, 2023 10:58
@EgorkaKulikov EgorkaKulikov self-requested a review June 1, 2023 11:02
@EgorkaKulikov EgorkaKulikov merged commit fd85321 into main Jun 1, 2023
@EgorkaKulikov EgorkaKulikov deleted the ilya_m/spring_integration_test_prototype branch June 1, 2023 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp-fuzzing Issue is related to the fuzzing comp-spring Issue is related to Spring projects support ctg-enhancement New feature, improvement or change request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants